Escape snapshot names in libvirt XML - #14201
Open
sureshanaparti wants to merge 1 commit into
Open
sureshanaparti wants to merge 1 commit into
sureshanaparti wants to merge 1 commit into
Conversation
Contributor
Author
|
@blueorangutan package |
|
@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #14201 +/- ##
============================================
+ Coverage 17.97% 17.98% +0.01%
- Complexity 16184 16191 +7
============================================
Files 5930 5930
Lines 535615 535615
Branches 65582 65582
============================================
+ Hits 96271 96341 +70
+ Misses 428377 428301 -76
- Partials 10967 10973 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
sureshanaparti
changed the title
sureshanaparti
force-pushed
the
escape-snapshot-names-libvirt
branch
from
September 18, 2026 12:38
131ee43 to
b8e990d
Compare
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19284 |
Contributor
|
@blueorangutan test |
|
@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
JoaoJandre
approved these changes
Sep 18, 2026
JoaoJandre
left a comment
JoaoJandre
left a comment
Contributor
There was a problem hiding this comment.
CLGTM, did not test it
|
[SF] Trillian test result (tid-17026)
|
kiranchavala
approved these changes
Sep 23, 2026
Member
There was a problem hiding this comment.
Before fix
- Take one memory snapshot of a running KVM VM
cmk create vmsnapshot virtualmachineid=<vm-uuid> snapshotmemory=true
cmk list vmsnapshots virtualmachineid=<vm-uuid> filter=id,name,type,state
- Check the snapshot state
virsh snapshot-list i-2-32-VM
Name Creation Time State
--------------------------------------------------------------------
i-2-32-VM_VS_20260923045048 2026-09-23 04:50:49 +0000 running
- Check the database
Databse (select * from vm_snapshots)
*************************** 2. row ***************************
id: 2
uuid: f6dd533b-94bb-4218-88dc-0d8ee5b2d4aa
name: i-2-32-VM_VS_20260923045048
display_name: test-snapshot
description: test-snapshot
vm_id: 32
account_id: 2
domain_id: 1
service_offering_id: 2
vm_snapshot_type: DiskAndMemory
state: Ready
parent: NULL
current: 1
update_count: 2
updated: 2026-09-23 04:50:56
created: 2026-09-23 04:50:48
removed: NULL
2 rows in set (0.00 sec)
- Update the database entry
mysql > UPDATE cloud.vm_snapshots SET name = CONCAT(name, '</name><description>INJECTED</description><name>x') WHERE uuid = 'f6dd533b-94bb-4218-88dc-0d8ee5b2d4aa';
- Stop the VM
cmk stop virtualmachine id=<vm-uuid>
- Start the VM. This sends RestoreVMSnapshotCommand to the host.
cmk start virtualmachine id=<vm-uuid>
- Check the KVM Agent log
[root@ref-trl-12298-k-Mr8-suresh-anaparti-kvm2 ~] grep -A8 "Restoring Instance Snapshot" /var/log/cloudstack/agent/agent.log | tail -20
2026-09-23 04:53:57,948 DEBUG [resource.wrapper.LibvirtRestoreVMSnapshotCommandWrapper] (AgentRequest-Handler-3:[]) (logid:e5d4f5ea) Restoring Instance Snapshot i-2-32-VM_VS_20260923045048</name><description>INJECTED</description><name>x on i-2-32-VM with XML:
<domainsnapshot>
<name>i-2-32-VM_VS_20260923045048</name><description>INJECTED</description><name>x</name>
<state>running</state>
<creationTime>1790139048</creationTime>
<domain type='kvm' id='18'>
<name>i-2-32-VM</name>
<uuid>9b044695-8d01-4265-84a5-667dec666ee5</uuid>
<description>CentOS 5.5 (64-bit)</description>
- Check the xml
virsh snapshot-dumpxml i-2-32-VM i-2-32-VM_VS_20260923045048 | head
<domainsnapshot>
<name>i-2-32-VM_VS_20260923045048</name>
<description>INJECTED</description>
<state>running</state>
<creationTime>1790139048</creationTime>
<memory snapshot='internal'/>
<disks>
<disk name='vda' snapshot='internal'/>
<disk name='hdc' snapshot='no'/>
<disk name='hdd' snapshot='no'/>
After fix
- Follow the same steps to create a snaphot
[root@ref-trl-12386-k-Mol8-kiran-chavala-kvm1 ~] virsh snapshot-list i-2-3-VM
Name Creation Time State
-------------------------------------------------------------------
i-2-3-VM_VS_20260923050607 2026-09-23 05:06:08 +0000 running
[root@ref-trl-12386-k-Mol8-kiran-chavala-kvm1 ~]# virsh snapshot-dumpxml i-2-3-VM i-2-3-VM_VS_20260923050607 |head
<domainsnapshot>
<name>i-2-3-VM_VS_20260923050607</name>
<state>running</state>
<creationTime>1790139968</creationTime>
<memory snapshot='internal'/>
<disks>
<disk name='vda' snapshot='internal'/>
<disk name='hdc' snapshot='no'/>
</disks>
<domain type='kvm'>
- Update the database
mysql> UPDATE cloud.vm_snapshots SET name = CONCAT(name, '</name><description>INJECTED</description><name>x') WHERE uuid ="66bfd611-f20b-446d-ae5f-a0938c90fe2f";
-
stop the vm
-
start the vm
[root@ref-trl-12386-k-Mol8-kiran-chavala-kvm1 ~]virsh snapshot-list i-2-3-VM
Name Creation Time State
-------------------------------
[root@ref-trl-12386-k-Mol8-kiran-chavala-kvm1 ~]# virsh snapshot-dumpxml i-2-3-VM i-2-3-VM_VS_20260923050607 |head
error: Domain snapshot not found: no domain snapshot with matching name 'i-2-3-VM_VS_20260923050607'
Check the KVM Agent log and Management server log
[root@ref-trl-12386-k-Mol8-kiran-chavala-kvm1 ~]# grep -A8 "Restoring Instance Snapshot" /var/log/cloudstack/agent/agent.log | tail -20
2026-09-23 05:11:18,143 DEBUG [resource.wrapper.LibvirtRestoreVMSnapshotCommandWrapper] (AgentRequest-Handler-4:[]) (logid:47d1f8a0) Restoring Instance Snapshot i-2-3-VM_VS_20260923050607</name><description>INJECTED</description><name>x on i-2-3-VM with XML:
<domainsnapshot>
<name>i-2-3-VM_VS_20260923050607</name><description>INJECTED</description><name>x</name>
<state>running</state>
<creationTime>1790139968</creationTime>
<domain type='kvm' id='5'>
<name>i-2-3-VM</name>
<uuid>c74b883d-eee4-489d-8f64-82de065c2f61</uuid>
<description>CentOS 5.5 (64-bit)</description>
2026-09-23 05:11:18,149 DEBUG [resource.wrapper.LibvirtRestoreVMSnapshotCommandWrapper] (AgentRequest-Handler-4:[]) (logid:47d1f8a0) Failed to restore Instance Snapshot i-2-3-VM_VS_20260923050607</name><description>INJECTED</description><name>x on i-2-3-VM
[root@ref-trl-12386-k-Mol8-kiran-chavala-mgmt1 ~]# 2026-09-23 05:11:18,197 WARN [c.c.v.UserVmManagerImpl] (Work-Job-Executor-6:[ctx-3e55c107, job-44/job-45, ctx-6020f68b]) (logid:47d1f8a0) Unable to restore the Instance Snapshot from image file to the Instance: org.libvirt.LibvirtException: invalid snapshot name 'i-2-3-VM_VS_20260923050607</name><description>INJECTED</description><name>x': name can't contain '/'
Test case 2 ( revertVmsnapshot)
-
Have a vm in running state
-
Take a vm snapshot
[root@ref-trl-12386-k-Mol8-kiran-chavala-kvm1 ~]# virsh snapshot-dumpxml i-2-3-VM i-2-3-VM_VS_20260923050607 |head
<domainsnapshot>
<name>i-2-3-VM_VS_20260923050607</name>
<state>running</state>
<creationTime>1790139968</creationTime>
<memory snapshot='internal'/>
<disks>
<disk name='vda' snapshot='internal'/>
<disk name='hdc' snapshot='no'/>
</disks>
<domain type='kvm'>
- Update the databse ( vm_snapshot)
mysql> UPDATE cloud.vm_snapshots SET name = CONCAT(name, '</name><description>INJECTED</description><name>x') WHERE uuid ="66bfd611-f20b-446d-ae5f-a0938c90fe2f";
- Revert Snapshot
Agent.log
2026-09-23 05:40:16,765 WARN [resource.wrapper.LibvirtRevertToVMSnapshotCommandWrapper] (AgentRequest-Handler-1:[]) (logid:57fe53eb) Revert to Instance Snapshot failed due to org.libvirt.LibvirtException: Domain snapshot not found: no domain snapshot with matching name 'i-2-3-VM_VS_20260923050607</name><description>INJECTED</description><name>x' org.libvirt.LibvirtException: Domain snapshot not found: no domain snapshot with matching name 'i-2-3-VM_VS_20260923050607</name><description>INJECTED</description><name>x'
at org.libvirt.ErrorHandler.processError(Unknown Source)
at org.libvirt.ErrorHandler.processError(Unknown Source)
at org.libvirt.Domain.snapshotLookupByName(Unknown Source)
Management server log
2026-09-23 05:40:16,849 ERROR [c.c.a.ApiAsyncJobDispatcher] (API-Job-Executor-40:[ctx-c12b5551, job-55]) (logid:57fe53eb) Unexpected exception while executing org.apache.cloudstack.api.command.admin.vmsnapshot.RevertToVMSnapshotCmdByAdmin com.cloud.utils.exception.CloudRuntimeException: Unable to revert VM i-2-3-VM to snapshot i-2-3-VM_VS_20260923050607</name><description>INJECTED</description><name>x.
at com.cloud.vm.snapshot.VMSnapshotManagerImpl.orchestrateRevertToVMSnapshot(VMSnapshotManagerImpl.java:988)
at com.cloud.vm.snapshot.VMSnapshotManagerImpl.orchestrateRevertToVMSnapshot(VMSnapshotManagerImpl.java:1344)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:569)
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR escapes the snapshot names in the three places that build the document: LibvirtCreateVMSnapshotCommandWrapper, generateVMSnapshotXML (snapshot and parent), and the MessageFormat argument in LibvirtManageSnapshotCommandWrapper.
The raw name still goes to snapshotLookupByName, which takes a plain string, not markup.
Uses commons-lang StringEscapeUtils.escapeXml, same as LibvirtVMDef does for the VNC password, so no new dependency. Create and restore escape identically, so existing snapshots are still found by name.
The KVM wrappers build documents by string concatenation and drop the snapshot name straight in.
A name carrying XML metacharacters could close the element early and add arbitrary children, such as a block pointing anywhere on the hypervisor.
Nothing reaching these sinks today can carry a metacharacter: on KVM the vm.instancename.flag path is gated on VMware, hostnames go through NetUtils.verifyDomainNameLabel, and KVMStorageProcessor uses random UUIDs.
This is hardening so the sink no longer depends on its callers.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?